Repository navigation
fix(ethernet,pcapng): reject a non-string MAC or EUI address (#2222) - #2232
Conversation
|
Review: GOOD TO GO at
|
8bffb29 to
0681653
Compare
|
Review: GOOD TO GO at
|
|
Coverage: 91.56% (unit tier, Python 3.15,
Per-file detail: the |
|
Ready to merge at
|
|
if we enforce str/bytes/bytearray, then why we'd still need the bool check? are we dropping the acceptance of |
|
No You're right that the
Recommendation: drop the |
|
take your recommendations and make sure the error message follows the conventions. |
|
Review: GOOD TO GO at
|
|
Ready to merge at
|
… ProtocolError (#2256) (#2257) ARP._make_addr_resolve raised unprefixed, capitalised messages ("Invalid MAC address: ...", "Invalid hardware address: ..."), gave a wrong type the same message as a malformed string, and rejected a bool with a separate FieldValueError that "except ProtocolError" missed. Drop the bool check, as #2232 did for Ethernet and PCAP-NG: any address that is not a str, bytes or bytearray raises "ARP: invalid MAC address: must be a str, bytes or bytearray, not ..." ("hardware address" for a non-Ethernet htype), and a malformed string raises "ARP: invalid MAC address: ...". Accepted forms are unchanged. The 1.5.0 changelog entry for #2166 and the generated CHANGELOG.md are corrected to match. Test: TestARPHardwareAddressType pins the exact messages for bool, int, None, float and a malformed string on Ethernet and Fibre Channel.
…2265) - #876 item 3: state why the no-else-raise pragma stays, not "the belief" - #652: the flagless HTTP/2 dump reads "Flags::0 [0]", as #648 renders it - #2172: section_context raises only on a block holding no section - add the missing #2222/#2232 MAC/EUI maker entry; cite #2257 on ARP - add the #1055/#2244 engine-tests and required-checks entry that #849's and #856's job descriptions now need - #800 names HTTP._guess_version, where the method lives; #631 states the old MANIFEST.in in the past tense - drop mentions of never-released names (pcapkit.utilities.timestamp, a test constant) - citation range runs to #2257; CHANGELOG.md regenerated
) No ProtocolError in pcapkit.protocols.internet.esp carried the "<prefix>: <lowercase description>: ... {value!r}" form #2232 and #2257 gave Ethernet, PCAP-NG and ARP: none had an "ESP:" prefix, some put "ESP" mid-sentence, and most printed the value bare. Every message raised by _resolve, CipherSuite.get, IntegritySuite.get, SecurityAssociation (SPI, destination, AEAD plus integrity, ICV length, compute_icv, decrypt, encrypt, _split_key), ESPContext.register and ESP.make now reads "ESP: <description>: ...", with the offending value in !r form. SecurityAssociation.unavailable gives the same prefixed reason. The traceback quoted in the module docstring and esp.rst follows. Test: ESPErrorMessageTests pins each message whole; the HAS_CRYPTO gate count in test_tier_guard rises by its one gated test.
IPv6_Ext's version gate raised "only valid for IPv6, got version=4" -- a
bespoke tail unlike the <prefix>: <lowercase description>: must be X, not
{value!r} form #2232/#2257 established, and it never named the version
that was wanted. The same method's docstring quoted the message verbatim
for the AH/ESP case, so it was stale by the same edit.
Message is now "invalid version: must be 6, not {version!r}".
Test: test_version_gate_message_is_in_the_house_form pins the message
whole on both routes into the gate -- a bare-bytes call and the
BytesIO/alias=/extension=True stream form.
…pe in option guards (#2398) #2300: the guards that reject a file's magic number or byte order had no PCAP:/PCAP-NG: prefix, unlike every other message in those modules, and PCAP's magic-number guard didn't say what it read. Four sites: schema/misc/pcap/header.py's magic_number_callback, misc/pcap/header.py's _make_magic (byte order unresolved/unknown), and two byte-order sites in schema/misc/pcapng.py that were the only two of nine messages in that file still missing PCAP-NG:. #2301: 128 PCAP-NG option guards in misc/pcapng.py ended in a full stop, unlike the house <prefix>: ... form (#2232, #2257), and the placement guard formatted self._type -- a plain int at make time, not the Enum_ BlockType it's cast to -- with bare {}, showing a raw number. A new _block_type_name() helper resolves either form through Enum_BlockType, idempotent for the enum case; applying the int form via .name directly would raise AttributeError, verified against pristine main. Also fixed: the if_tsresol/if_tzone option guards carried the identical full-stop defect, outside the issue's literal checklist but left would have made this consistency fix inconsistent with itself -- the file's full-stop count is now 0, which a new test asserts as an invariant. CHANGELOG.md regenerated (util/changelog_md.py) after correcting one docs/source/changelog/1.5.0.rst entry that quoted the old literal message. Tests: new tests/protocols/misc/test_capture_message_prefix_2300_2301_unit.py, 14 cases, 13 fail / 1 passes against pristine main (the one passing case is a characterisation pin of _type's runtime type, true on both trees).
…ne_name__ (#2399) The pcapkit.toolkit adapters put the engine name mid-sentence instead of leading with it, unlike the <prefix>: <lowercase description>: ... {value!r} form #2232/#2257 established, and hand-typed the name rather than reading it off the engine class's own __engine_name__. Each adapter module gains _ENGINE = <EngineClass>.__engine_name__ (plus the import), so a rename in pcapkit/foundation/engines/ cannot leave the adapter spelling the old name. Four modules, five sub-bugs: - pcap_ct.py/pypcap.py: "IPv4 reassembly is not supported by the pcap-ct engine: ..." -> "PCAP_CT: IPv4 reassembly is not supported: ..." (and the PyPCAP sibling). - pypcapfile.py: "invalid PyPCAPFile IPv4 address: 5" -> "PyPCAPFile: invalid IPv4 address: 5" (same for MAC address); the IPv6 UnsupportedCall and the "Frame N: TCP over IPv6 ..." warning get the same treatment. - pyshark.py: boolean-field, frame.time_epoch and no-frame-layer messages all reordered the same way. - scapy.py: the import-time warning reordered; the quoted package name is lowercased to 'scapy' to match the actual failing import and the sibling dependency warnings elsewhere in the codebase, which quote the installable name. The engine classes' toolkit imports are all function-local, so the new toolkit -> foundation.engines import introduces no cycle. Tests: new tests/toolkit/test_engine_message_prefix_2304_unit.py (9 tests) pins each message whole and asserts each module's _ENGINE is the engine class's own attribute. Existing pins in the four engines' test files updated for the new text.
make testpasses, and a test case covers the changeWhat is the purpose of your pull request?
fix— bug fixDescription
Closes #2222.
Ethernet._make_mac_addrsent a non-string address straight toPAT_MAC_ADDR.fullmatch, soEthernet(src=True),dst=5,src=Noneanddst=1.5raised a bareTypeError.str,bytesorbytearray, aboolincluded, raisesProtocolError. Every message in the three makers carries the protocol prefix (Ethernet: invalid MAC address: must be a str, bytes or bytearray, not True).intstays unaccepted, as in every release.tests/protocols/link/test_ethernet_mac_type_unit.pypins the message for bool, int, None and float oversrcanddst, and for a malformed string, and checks thatstr,bytesandbytearraystill pack.PCAPNG._make_mac_addrandPCAPNG._make_eui_addrhad the same gap and get the same check and prefix. Their only callers are the IDBif_MACaddrandif_EUIaddroption builders, whichtests/protocols/misc/test_pcapng_addr_type_2222_unit.pydrives.origin/main, the two test files give 23 failed: the bareTypeError, and the unprefixed malformed-string message.tests/protocols/linkgives 112 passed,tests/protocols/misc/test_pcapng*211 passed, and the docstring contract and tier guard 140 passed.